feat(tui): enhance approval modal button prominence with visual grouping by donglovejava · Pull Request #3437 · Hmbown/CodeWhale

donglovejava · 2026-06-23T03:26:46Z

Summary

This PR enhances the approval modal UX by making the approve/deny buttons more visually prominent and easier to distinguish.

Problem

The approval modal displayed all four options (Approve once, Approve always, Deny, Abort) as a flat list with no visual grouping. Users had to read each line carefully to distinguish between approve and deny actions. The selected option was only indicated by a color change, which could be missed at a glance.

Solution

Visual Grouping

Added a separator line between the approve group (options 0-1) and the deny/abort group (options 2-3), making the decision surface read as two distinct choice clusters.

Button-Like Selected Row

The selected option now renders as a solid button row with a full background strip using SELECTION_BG.
Added an ◄ EXECUTE indicator on the right side of the selected row, giving clear feedback about which action will fire on Enter.

Keyboard Shortcut Emphasis

Maintained BOLD modifier on shortcut hints for both selected and unselected rows.

Before vs After

Before:

  [1 / y] Approve once
  [2 / a] Approve always for this kind
  [3 / d / n] Deny this call
  [Esc] Abort the turn

After:

  [1 / y] Approve once
  [2 / a] Approve always for this kind
  ──────────────────────────────────────────
  [3 / d / n] Deny this call
  [Esc] Abort the turn

Selected row now shows:

  [1 / y] Approve once              ◄ EXECUTE

Impact

Users can instantly identify the approve vs deny groups
The selected action is unmistakable with the background strip and EXECUTE indicator
Reduces accidental approvals/denials from misreading the option list

Generated with Claude Code

The sidebar was only showing when terminal width >= 100 columns, which is too restrictive for many terminal setups. Reduced the minimum width to 60 columns to make the sidebar visible in more common terminal configurations. This fixes the issue where the sidebar would not appear in v0.8.62+ when using typical terminal sizes that are narrower than 100 columns.

Nightly builds: - Add artifact existence check to skip redundant builds for the same commit - Add build retry logic (up to 3 attempts) for transient failures - Add nightly-complete summary job for branch protection rules - Improve concurrency group to use ref_name instead of full ref Auto-tag idempotency: - Add semver validation for workspace version - Add annotated tags with release metadata - Add push retry logic with exponential backoff - Fail fast if version consistency check fails - Add concurrency control to prevent race conditions Addresses v0.8.64 reliability concerns for nightly builds and auto-tagging.

Update SECURITY.md email address from the legacy deepseek-tui.com domain to codewhale.com to match the project rebranding. Addresses v0.8.64 security hardening requirements.

- Add visual separator between approve (0-1) and deny/abort (2-3) groups - Render selected option as a solid button row with background strip - Add 'EXECUTE' indicator on the selected row for clear action feedback - Maintain keyboard shortcut emphasis with BOLD modifier Improves UX by making the decision surface read as two distinct choice clusters rather than a flat list, and gives the selected option a clear button-like appearance.

greptile-apps

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

gemini-code-assist

Code Review

This pull request introduces several updates, including adding new native tools to the tool catalog, reducing the minimum sidebar width, and enhancing the 'ApprovalWidget' with visual separators and selection highlights. It also includes several documentation files and a helper script ('fix_engine.py'). The code review feedback suggests improving the 'ApprovalWidget' layout by dynamically calculating the separator line width to prevent wrapping, padding the selected option background to span the full card width, and removing the unused 'locale_separator' helper. Additionally, it recommends replacing the hardcoded absolute Windows path in 'fix_engine.py' with a relative path for portability.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

gemini-code-assist · 2026-06-23T03:28:47Z

+        let separator = locale_separator(locale);

        for (i, opt) in options.iter().enumerate() {
+            // Insert a visual separator between the approve group (0-1)
+            // and the deny/abort group (2-3) so the decision surface
+            // reads as two distinct choice clusters rather than a flat
+            // list.
+            if i == 2 {
+                lines.push(Line::from(vec![Span::styled(
+                    separator,
+                    Style::default().fg(palette::TEXT_MUTED),
+                )]));
+            }


The separator line is currently hardcoded to 40 characters. However, the approval card width is dynamic and can be as small as APPROVAL_CARD_MIN_WIDTH (40), which results in a content width of 36. In such cases, the 40-character separator line will wrap to the next line, breaking the visual layout.

We can dynamically construct the separator line using card_area.width to ensure it perfectly spans the available content width without wrapping.

Suggested change

let separator = locale_separator(locale);

for (i, opt) in options.iter().enumerate() {

// Insert a visual separator between the approve group (0-1)

// and the deny/abort group (2-3) so the decision surface

// reads as two distinct choice clusters rather than a flat

// list.

if i == 2 {

lines.push(Line::from(vec![Span::styled(

separator,

Style::default().fg(palette::TEXT_MUTED),

)]));

}

let content_width = card_area.width.saturating_sub(4) as usize;

for (i, opt) in options.iter().enumerate() {

// Insert a visual separator between the approve group (0-1)

// and the deny/abort group (2-3) so the decision surface

// reads as two distinct choice clusters rather than a flat

// list.

if i == 2 {

let sep_chars = "─".repeat(content_width.saturating_sub(2));

lines.push(Line::from(vec![Span::styled(

format!(" {sep_chars}"),

Style::default().fg(palette::TEXT_MUTED),

)]));

}

gemini-code-assist · 2026-06-23T03:28:47Z

+            if is_selected {
+                // Selected option: render as a solid button row with a
+                // distinct background strip so the user can instantly see
+                // which action will fire on Enter.
+                lines.push(Line::from(vec![
+                    Span::styled("  ", Style::default().bg(palette::SELECTION_BG)),
+                    Span::styled(
+                        format!("[{}] ", opt.key_hint),
+                        shortcut_style.add_modifier(Modifier::BOLD).bg(palette::SELECTION_BG),
+                    ),
+                    Span::styled(
+                        opt.label.to_string(),
+                        option_style.add_modifier(Modifier::BOLD).bg(palette::SELECTION_BG),
+                    ),
+                    Span::styled("  ", Style::default().bg(palette::SELECTION_BG)),
+                    Span::styled(
+                        selected_indicator(locale),
+                        Style::default()
+                            .fg(palette::SELECTION_TEXT)
+                            .bg(palette::SELECTION_BG)
+                            .add_modifier(Modifier::BOLD),
+                    ),
+                ]));


The selected option background is currently applied only to the individual text spans. This means the background highlight will only cover the text itself, rather than forming a solid, full-width button row as described in the PR. Additionally, the ◄ EXECUTE indicator is placed immediately after the label instead of being right-aligned to the edge of the card.

We can resolve both issues by calculating the remaining width of the row and inserting a padding span of spaces with the SELECTION_BG background between the label and the indicator. This right-aligns the indicator and extends the background highlight across the entire width of the card.

if is_selected { // Selected option: render as a solid button row with a // distinct background strip so the user can instantly see // which action will fire on Enter. let shortcut_text = format!("[{}] ", opt.key_hint); let label_text = opt.label.to_string(); let indicator_text = selected_indicator(locale); let text_width = 2 + shortcut_text.width() + label_text.width() + indicator_text.width(); let padding_width = content_width.saturating_sub(text_width); lines.push(Line::from(vec![ Span::styled(" ", Style::default().bg(palette::SELECTION_BG)), Span::styled( shortcut_text, shortcut_style.add_modifier(Modifier::BOLD).bg(palette::SELECTION_BG), ), Span::styled( label_text, option_style.add_modifier(Modifier::BOLD).bg(palette::SELECTION_BG), ), Span::styled( " ".repeat(padding_width), Style::default().bg(palette::SELECTION_BG), ), Span::styled( indicator_text, Style::default() .fg(palette::SELECTION_TEXT) .bg(palette::SELECTION_BG) .add_modifier(Modifier::BOLD), ), ])); } else {

gemini-code-assist · 2026-06-23T03:28:47Z

+fn locale_separator(locale: Locale) -> &'static str {
+    match locale {
+        Locale::ZhHans => "  ────────────────────────────────────────",
+        _ => "  ────────────────────────────────────────",
+    }
+}


Since the separator line is now constructed dynamically based on the card width, the locale_separator helper function is no longer used and can be safely removed.

gemini-code-assist · 2026-06-23T03:28:47Z

@@ -0,0 +1,39 @@
+import re
+
+file_path = r'C:\project\F_project1\CodeWhale\crates\tui\src\core\engine.rs'


The script uses a hardcoded absolute Windows path (C:\project\F_project1\...). This will fail on other developers' machines or in CI environments.

Using a relative path makes the script portable across different environments and operating systems.

Suggested change

file_path = r'C:\project\F_project1\CodeWhale\crates\tui\src\core\engine.rs'

file_path = 'crates/tui/src/core/engine.rs'

Hmbown · 2026-06-23T04:36:55Z

I love this!! I've been fighting getting this down so your assistance is so appreciated.

Hmbown · 2026-06-23T18:57:59Z

Thanks @donglovejava — the approval-modal grouping idea is good (separator between the approve cluster and the deny/abort cluster, plus a clear indicator of which row fires on Enter). But this PR can't land as-is, and the salvageable part needs a small rework. Concrete path to a mergeable single-file PR:

Keep only crates/tui/src/tui/widgets/mod.rs (the ApprovalWidget render). Drop everything else from the branch:

SECURITY.md — this regresses the security contact from the current security@codewhale.net (on main) back to @codewhale.com. Must not land.
codewhale — a stray submodule gitlink (5dffecef…) with no .gitmodules; would break clones.
fix_engine.py, *.patch, and the 7 pr-body-*.md scratch files — repo pollution.
.github/workflows/auto-tag.yml / nightly.yml — release-tagging changes need Hunter's explicit approval and shouldn't ride in a TUI feature PR.
tool_catalog.rs (exec_*/task_shell_*) and the SIDEBAR_VISIBLE_MIN_WIDTH change — already superseded on main (stale base).

Rework the widget hunk before it compiles/ships:

It references palette::SELECTION_BG, which does not exist on main. Selection highlight is theme-aware via approval_option_style() / approval_selected_style() and the per-theme selection backgrounds (Whale/Light/Grayscale/Matrix/Solarized). Hardcoding one BG would break the other themes. Please route the selected-button styling through the existing theme-aware accessor instead.
locale_separator() returns the same 40-dash string for both ZhHans and the default arm — collapse it (and bound the width so it doesn't wrap on narrow terminals).
selected_indicator() ( ◄ EXECUTE / ◄ 执行) is fine.

The grouped-button restyle is also an aesthetic change to the security-critical approval surface, so I'm flagging it for Hunter's design sign-off. If you'd like, open a fresh branch off current main with just the reworked widget change and I'll review/credit it. Appreciate the contribution.

@donglovejava

…ow caret Reduce approve/deny misclicks in the approval takeover: - draw a divider between the approve group (once / always-for-session) and the deny/abort group so they read as two distinct decisions - mark the highlighted row with a leading caret so the action Enter will fire is unmistakable, not signalled by the selection background alone Locale-neutral (no new MessageId), reusing the existing selection style. The divider is sized to fit the minimum card inner width; a render test pins the caret and divider glyphs. The four decisions, footer affordances (v / s / Esc), and the session-vs-persistent approval behavior are unchanged. Harvested from PR #3437 by @donglovejava (the group-separator + selected-row idea), reapplied onto current main in the simplest form and trimmed of that branch's unrelated churn (tool_catalog edits, workflow/SECURITY.md changes, stray files). Co-authored-by: donglovejava <211940267+donglovejava@users.noreply.github.com> Claude-Session: https://claude.ai/code/session_01991fnUqBbWSgiUFw33L8XX

@donglovejava

…ow caret (#3515) Reduce approve/deny misclicks in the approval takeover: - draw a divider between the approve group (once / always-for-session) and the deny/abort group so they read as two distinct decisions - mark the highlighted row with a leading caret so the action Enter will fire is unmistakable, not signalled by the selection background alone Locale-neutral (no new MessageId), reusing the existing selection style. The divider is sized to fit the minimum card inner width; a render test pins the caret and divider glyphs. The four decisions, footer affordances (v / s / Esc), and the session-vs-persistent approval behavior are unchanged. Harvested from PR #3437 by @donglovejava (the group-separator + selected-row idea), reapplied onto current main in the simplest form and trimmed of that branch's unrelated churn (tool_catalog edits, workflow/SECURITY.md changes, stray files). Claude-Session: https://claude.ai/code/session_01991fnUqBbWSgiUFw33L8XX Co-authored-by: donglovejava <211940267+donglovejava@users.noreply.github.com>

github-actions · 2026-06-24T00:12:23Z

Thanks @donglovejava — your contribution landed in a068f81161bd on main:

feat(tui): clarify approval modal with a group divider and selected-row caret (#3515)

Closing this PR now that the code is on main. Credit lives in the commit message and (where applicable) the CHANGELOG.md entry for the next release. Apologies for not closing this at the time of the merge — the auto-close workflow is new in v0.8.31.

If you want to land more work and would prefer your future PRs merge cleanly without a harvest step, the CONTRIBUTING.md doc has a short note on what makes a contribution mergeable as-is.

donglovejava added 5 commits May 29, 2026 06:47

fix(tools): eagerly load all exec_shell companion tools

d7b2983

fix(security): update security contact email to codewhale.com

305b9e3

Update SECURITY.md email address from the legacy deepseek-tui.com domain to codewhale.com to match the project rebranding. Addresses v0.8.64 security hardening requirements.

donglovejava requested a review from Hmbown as a code owner June 23, 2026 03:26

greptile-apps Bot reviewed Jun 23, 2026

View reviewed changes

gemini-code-assist Bot reviewed Jun 23, 2026

View reviewed changes

This was referenced Jun 23, 2026

docs: add official-site and mirror provenance copy to codewhale.net #3440

Closed

docs: reconcile v0.8.65 release ledger to live state #3493

Open

feat(tui): clarify approval modal (harvest of #3437 by @donglovejava) #3515

Merged

github-actions Bot closed this Jun 24, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(tui): enhance approval modal button prominence with visual grouping#3437

feat(tui): enhance approval modal button prominence with visual grouping#3437
donglovejava wants to merge 5 commits into
Hmbown:mainfrom
donglovejava:fix/approval-modal-button-prominence

donglovejava commented Jun 23, 2026

Uh oh!

greptile-apps Bot left a comment

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Jun 23, 2026

Uh oh!

gemini-code-assist Bot Jun 23, 2026

Uh oh!

gemini-code-assist Bot Jun 23, 2026

Uh oh!

gemini-code-assist Bot Jun 23, 2026

Uh oh!

Hmbown commented Jun 23, 2026

Uh oh!

Hmbown commented Jun 23, 2026

Uh oh!

github-actions Bot commented Jun 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -0,0 +1,39 @@
		import re

		file_path = r'C:\project\F_project1\CodeWhale\crates\tui\src\core\engine.rs'

	file_path = r'C:\project\F_project1\CodeWhale\crates\tui\src\core\engine.rs'
	file_path = 'crates/tui/src/core/engine.rs'

Conversation

donglovejava commented Jun 23, 2026

Summary

Problem

Solution

Visual Grouping

Button-Like Selected Row

Keyboard Shortcut Emphasis

Before vs After

Impact

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

Hmbown commented Jun 23, 2026

Uh oh!

Hmbown commented Jun 23, 2026

Uh oh!

github-actions Bot commented Jun 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants